Skip to content

Allow slice syntax in e.g. Annotated[int, 1:3] and TensorType["batch":..., float] #11345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 18, 2021
Merged

Allow slice syntax in e.g. Annotated[int, 1:3] and TensorType["batch":..., float] #11345

merged 2 commits into from
Oct 18, 2021

Conversation

@Zac-HD
Copy link
Contributor

@Zac-HD Zac-HD commented Oct 16, 2021 •
edited
Loading

Consider the following trivial Python file, using the torchtyping library - if only this typechecked, we could write some fancy ML code!

from typing import Annotated, Dict
from torchtyping import TensorType  # type: ignore

a: Annotated[int, 1:2]
b: Dict[int, x:y]
c: Dict[x:y]
t: TensorType["batch":..., float]

On master, we get syntax-level errors due to the use of : slice syntax, which prevents any semantic checking - and can't even be disabled by a config file!

$ mypy --show-column-numbers t.py
t.py:4:4: error: Slice usage in type annotation is invalid
t.py:5:4: error: Slice usage in type annotation is invalid
t.py:6:4: error: Slice usage in type annotation is invalid
t.py:7:4: error: Slice usage in type annotation is invalid
Found 4 errors in 1 file (errors prevented further checking)

Following this patch, mypy identifies the problems with our Dict annotations, but is happy with Annotated and can be directed to ignore TensorType:

$ mypy --show-column-numbers t.py
t.py:5:14: error: Invalid type comment or annotation
t.py:5:14: note: did you mean to use ',' instead of ':' ?
t.py:6:4: error: "dict" expects 2 type arguments, but 1 given
t.py:6:9: error: Invalid type comment or annotation
t.py:6:9: note: did you mean to use ',' instead of ':' ?
Found 3 errors in 1 file (checked 1 source file)

This PR therefore fixes #10266; and closes #11279. Thanks to @sobolevn for pointing me in the right direction!

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
sobolevn
sobolevn reviewed Oct 16, 2021
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except that there's some weird interaction on Python < 3.9, which I don't understand well enough to track down quickly. Any suggestions?

Python3.9 has different ast.Subscript structure. This helper might be useful to understand how it changed: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/compat/functions.py#L32-L45

@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch from 020223b to af0e198 Compare October 16, 2021 09:57
@Zac-HD

This comment has been minimized.

Sign in to view
@sobolevn
Copy link
Member

sobolevn commented Oct 16, 2021

I get a different ordering of messages for same-line errors on different Python versions

Try to run your tests with --show-column-numbers, I suspect that it might be the issue. Since all three items are on the same line:

  main:3: error: "dict" expects 2 type arguments, but 1 given  [type-arg]
  main:3: error: Invalid type comment or annotation  [valid-type]
  main:3: note: did you mean to use ',' instead of ':' ?

Column numbers might be different here: https://github.com/python/mypy/pull/11345/files#diff-a7d2c40f4be6322cee498432ca9dbbc3a356c3cbcb61029231452f6638bb11b5R1565-R1572

@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch 6 times, most recently from 1cd724d to e095098 Compare October 17, 2021 11:51
@Zac-HD
Copy link
Contributor Author

Zac-HD commented Oct 17, 2021

Alright, I've fixed the pre-Python-3.9 issues: with message ordering (by propagating column numbers), and with Index nodes so that the int in Annotated[int, 3:4] is still recognised 🎉

However... this causes a problem, where on Python 3.9 mypyc complains that mypy/fastparse.py:1558: error: "Index" has no attribute "value". On one level this is fair enough - Index nodes are deprecated - but I can't even # type: ignore the problem because that's an unused ignore on older versions! Any further ideas @sobolevn?

@sobolevn
Copy link
Member

sobolevn commented Oct 17, 2021

@Zac-HD will conditional type alias work in this case? 🤔

if sys.version_info >= (3, 9):
    AstIndex = Any
else:
   AstIndex = ast.Index

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Oct 18, 2021

Or just imitate ASTConvertor.visit_index with return self.visit(cast(Any, n).value)? Let's see...

@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch from e095098 to efcd2a9 Compare October 18, 2021 00:20
@Zac-HD
Copy link
Contributor Author

Zac-HD commented Oct 18, 2021

Woohoo, it's all working! Thanks for your help @sobolevn 🥰

Ping @JelleZijlstra for review?

JelleZijlstra
JelleZijlstra reviewed Oct 18, 2021
View reviewed changes
JelleZijlstra
JelleZijlstra reviewed Oct 18, 2021
View reviewed changes
@Zac-HD
Allow slice syntax
22e8e3a 
This is useful for Annotated, and crucial for downstream libraries like torchtyping.
@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch from efcd2a9 to 22e8e3a Compare October 18, 2021 05:15
@Zac-HD Zac-HD requested a review from JelleZijlstra October 18, 2021 06:36
@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch 2 times, most recently from 619cf28 to 83ee297 Compare October 18, 2021 20:48
JelleZijlstra
JelleZijlstra reviewed Oct 18, 2021
View reviewed changes
@Zac-HD Zac-HD force-pushed the allow-slice-syntax branch from 83ee297 to a9f151c Compare October 18, 2021 21:52
@JelleZijlstra JelleZijlstra merged commit 9aaeef5 into python:master Oct 18, 2021
@Zac-HD Zac-HD deleted the allow-slice-syntax branch October 18, 2021 23:18
@Zac-HD Zac-HD mentioned this pull request Oct 29, 2021
Merged
ilevkivskyi pushed a commit that referenced this pull request Oct 29, 2021
@Zac-HD
Allow slice syntax (#11406)
e4c52ee 
This is useful for Annotated, and crucial for downstream libraries like torchtyping.

This PR cherrypicks #11345 onto the 0.920 release branch
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
@Zac-HD @tushar-deepsource
Allow slice syntax in e.g. Annotated[int, 1:3] and `TensorType["bat…
ebd6586 
…ch":..., float]` (python#11345)

This is useful for Annotated, and crucial for downstream libraries like torchtyping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@sobolevn sobolevn sobolevn left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Crash with slices e.g. Annotated[int, 3:4]

3 participants

@Zac-HD @sobolevn @JelleZijlstra